Feralfile Exhibition V4 #19

tienngovan · 2023-06-05T15:52:09Z

No description provided.

contracts/FeralfileArtworkV4.sol

…tract Remove ipfs and add vault contract

Send remaining funds to cost addr

lemonlatte · 2023-06-26T03:05:49Z

contracts/FeralfileArtworkV4.sol

+    /// @param tokenIds an array of token ID
+    function burnArtworks(uint256[] memory tokenIds) external {
+        for (uint256 i = 0; i < tokenIds.length; i++) {
+            _burnArtwork(tokenIds[i]);


Suggested change

_burnArtwork(tokenIds[i]);

require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721: caller is not token owner or approved");

_burnArtwork(tokenIds[i]);

@jollyjoker992 the external function can be called for users so we need to check the ownership before burn tokens.

lemonlatte

@jollyjoker992

lemonlatte · 2023-06-26T04:02:16Z

contracts/FeralfileArtworkV4.sol

-            interfaceId == type(IERC721Enumerable).interfaceId ||
-            super.supportsInterface(interfaceId);
+    /// @notice Stop token selling and transfer remaining tokens back to the underlying addresses
+    function stopSaleAndTransfer(


There is a chance to stop the sale with only part of series transfer tokens to addresses. This means the sale can start with the remaining tokens again. I think we need to ensure either:

the length of seriesIds is same to the length of _seriesMaxSupplies

balanceOf(address(this)) == 0

The first approach would need to introduce another key since we can not count the length of a map directly.

The later one would waste more gas since we can only check after we burn all tokens.

lemonlatte

LGTM

lpopo0856 · 2023-06-27T03:40:31Z

contracts/FeralfileArtworkV4.sol

+        address signer_,
+        bool burnable_,
+        bool bridgeable_,
+        address vault_,
+        address costReceiver_,


Small: put the same type params in order. though it wont save gas in function param, but it's better for reading

@lpopo0856 I dont think so, we should put the params based on the meaning like something is similar could put together. In this case, i will bring the signer next to vault and cost receiver.

lpopo0856 · 2023-06-27T03:45:57Z

contracts/FeralfileArtworkV4.sol

+    }
+
+    /// @notice Set vault contract
+    /// @dev don't allow to set signer as zero address


typo: set vault

lpopo0856 · 2023-06-27T03:50:44Z

contracts/FeralfileArtworkV4.sol

+        resumeSale();
+    }
+
+    /// @notice Resume token selling


small: consistent wording between selling and sale

lpopo0856 · 2023-06-27T03:52:12Z

contracts/FeralfileArtworkV4.sol

+        _selling = false;
+    }
+
+    /// @notice Stop token selling and burn remaining tokens


lpopo0856 · 2023-06-27T03:53:47Z

contracts/FeralfileArtworkV4.sol

+        }
+    }
+
+    /// @notice Stop token selling and transfer remaining tokens back to the underlying addresses


lpopo0856 · 2023-06-27T04:02:20Z

contracts/FeralfileArtworkV4.sol

+    /// @notice the cost receiver address
+    /// @param costReceiver_ - the address of cost receiver
+    function setCostReceiver(address costReceiver_) external onlyOwner {
+        costReceiver = costReceiver_;


missing costReceiver_ != address(0) check

lpopo0856 · 2023-06-27T04:03:17Z

contracts/FeralfileArtworkV4.sol

-    function setTokenBaseURI(string memory baseURI_) external onlyAuthorized {
-        _tokenBaseURI = baseURI_;
+    function setTokenBaseURI(string memory baseURI_) external onlyOwner {
+        tokenBaseURI = baseURI_;


missing bytes(baseURI_).length > 0 check

lpopo0856 · 2023-06-27T04:19:57Z

contracts/FeralfileArtworkV4.sol

+            for (uint16 j = 0; j < seriesIds.length; j++) {
+                if (artwork.seriesId == seriesIds[j]) {
+                    address to = recipientAddresses[j];
+                    _transfer(from, to, tokenId);


why not using _safeTransfer?

lemonlatte

LGTM

tienngovan force-pushed the FF_V4 branch from c0c15df to c2109cd Compare June 5, 2023 15:59

Implemented buyArtworks function

5af4d3b

tienngovan force-pushed the FF_V4 branch from c2109cd to 5af4d3b Compare June 5, 2023 16:08

Fixed prettier issue

8b02210

lemonlatte suggested changes Jun 6, 2023

View reviewed changes

contracts/FeralfileArtworkV4.sol Outdated Show resolved Hide resolved

contracts/FeralfileArtworkV4.sol Show resolved Hide resolved

contracts/FeralfileArtworkV4.sol Outdated Show resolved Hide resolved

contracts/FeralfileArtworkV4.sol Outdated Show resolved Hide resolved

lpopo0856 force-pushed the FF_V4 branch from eb82bf9 to 8b02210 Compare June 6, 2023 10:30

Implemented buy artwork with paid by vault contract

caaf8c1

tienngovan force-pushed the FF_V4 branch from c32a41d to caaf8c1 Compare June 6, 2023 11:59

lpopo0856 and others added 5 commits June 6, 2023 19:22

Remove ipfs from mint & add vault contract support

875a9af

better test language

1df88f3

add test for vault

46a4fa1

fixed conflict

7e8c13a

fixed revenue and cost

043d054

lemonlatte force-pushed the FF_V4 branch 2 times, most recently from ac4b47c to 08c7af9 Compare June 7, 2023 03:06

Add prettier github action

3c663e8

lemonlatte force-pushed the FF_V4 branch from 08c7af9 to 3c663e8 Compare June 7, 2023 03:08

lpopo0856 and others added 13 commits June 7, 2023 12:09

Fix the cost transfer logic

d468d6d

Better cost sending condition

9521dba

Merge pull request #20 from bitmark-inc/remove-ipfs-and-add-vault-con…

39c2475

…tract Remove ipfs and add vault contract

send remaining funds to cost addr

f6e7848

Merge pull request #22 from bitmark-inc/handle-remaining-funds

e48f7fc

Send remaining funds to cost addr

add vault contract signature validation

b3b0f98

make signer private

6ce78d2

Fix signer logic

0ef7594

adding paid lock

70d8477

adding withdraw function to exhibition contract

1b22ff8

update gitginore

97e06a8

Add prettier github action

40ad906

add mint functions

cf97c81

jollyjoker992 added 2 commits June 21, 2023 09:50

don't allow to send token to the contract

842e9c4

add some transfer tests

4d1ca3c

lemonlatte requested review from lemonlatte, jollyjoker992 and lpopo0856 June 21, 2023 03:52

jollyjoker992 added 2 commits June 21, 2023 10:59

fix wrong condition for stop sale and transfer

df9c093

update error message

c24e4fc

lemonlatte suggested changes Jun 26, 2023

View reviewed changes

add more checks for stopSaleAndTransfer and burnArtworks

8b8ef33

jollyjoker992 requested a review from lemonlatte June 26, 2023 16:17

lemonlatte previously approved these changes Jun 27, 2023

View reviewed changes

lpopo0856 reviewed Jun 27, 2023

View reviewed changes

add some validations

083f1f4

jollyjoker992 dismissed lemonlatte’s stale review via 083f1f4 June 27, 2023 07:30

lpopo0856 previously approved these changes Jun 27, 2023

View reviewed changes

jollyjoker992 previously approved these changes Jun 27, 2023

View reviewed changes

remove tokenBaseURI from constructor, fix migration scripts

78295a7

jollyjoker992 dismissed stale reviews from lpopo0856 and themself via 78295a7 June 27, 2023 10:28

lemonlatte approved these changes Jun 29, 2023

View reviewed changes

lemonlatte merged commit ff80a5d into main Jun 29, 2023
2 checks passed

lemonlatte deleted the FF_V4 branch June 29, 2023 02:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feralfile Exhibition V4 #19

Feralfile Exhibition V4 #19

tienngovan commented Jun 5, 2023

lemonlatte Jun 26, 2023

lemonlatte left a comment

lemonlatte Jun 26, 2023 •

edited

Loading

lemonlatte left a comment

lpopo0856 Jun 27, 2023 •

edited

Loading

jollyjoker992 Jun 27, 2023

lpopo0856 Jun 27, 2023

lpopo0856 Jun 27, 2023

lpopo0856 Jun 27, 2023

lpopo0856 Jun 27, 2023

lpopo0856 Jun 27, 2023

lpopo0856 Jun 27, 2023 •

edited

Loading

lpopo0856 Jun 27, 2023 •

edited

Loading

lpopo0856 Jun 27, 2023 •

edited

Loading

lemonlatte left a comment

	_burnArtwork(tokenIds[i]);
	require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721: caller is not token owner or approved");
	_burnArtwork(tokenIds[i]);

Feralfile Exhibition V4 #19

Feralfile Exhibition V4 #19

Conversation

tienngovan commented Jun 5, 2023

lemonlatte Jun 26, 2023

Choose a reason for hiding this comment

lemonlatte left a comment

Choose a reason for hiding this comment

lemonlatte Jun 26, 2023 • edited Loading

Choose a reason for hiding this comment

lemonlatte left a comment

Choose a reason for hiding this comment

lpopo0856 Jun 27, 2023 • edited Loading

Choose a reason for hiding this comment

jollyjoker992 Jun 27, 2023

Choose a reason for hiding this comment

lpopo0856 Jun 27, 2023

Choose a reason for hiding this comment

lpopo0856 Jun 27, 2023

Choose a reason for hiding this comment

lpopo0856 Jun 27, 2023

Choose a reason for hiding this comment

lpopo0856 Jun 27, 2023

Choose a reason for hiding this comment

lpopo0856 Jun 27, 2023

Choose a reason for hiding this comment

lpopo0856 Jun 27, 2023 • edited Loading

Choose a reason for hiding this comment

lpopo0856 Jun 27, 2023 • edited Loading

Choose a reason for hiding this comment

lpopo0856 Jun 27, 2023 • edited Loading

Choose a reason for hiding this comment

lemonlatte left a comment

Choose a reason for hiding this comment

lemonlatte Jun 26, 2023 •

edited

Loading

lpopo0856 Jun 27, 2023 •

edited

Loading

lpopo0856 Jun 27, 2023 •

edited

Loading

lpopo0856 Jun 27, 2023 •

edited

Loading

lpopo0856 Jun 27, 2023 •

edited

Loading